Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: disable IOReaderFactory for streaming requests #896

Merged

Conversation

happyalu
Copy link
Contributor

@happyalu happyalu commented Mar 6, 2019

An IOReaderFactory was being used to wrap request body for client/bidi streaming
requests. This was causing the requests to be fully buffered before being sent
to the grpc server, thereby breaking streaming. This commit changes that to
directly use request body.

Fixes #894

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix! Just a small comment.

protoc-gen-grpc-gateway/gengateway/template_test.go Outdated Show resolved Hide resolved
@johanbrandhorst
Copy link
Collaborator

Could you try building the grpc-gateway generator off this branch and see if it fixes tmc/grpc-websocket-proxy#14 for you?

An IOReaderFactory was being used to wrap request body for client/bidi streaming
requests.  This was causing the requests to be fully buffered before being sent
to the grpc server, thereby breaking streaming.  This commit changes that to
directly use request body.
@codecov-io
Copy link

Codecov Report

Merging #896 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #896   +/-   ##
=======================================
  Coverage   53.69%   53.69%           
=======================================
  Files          39       39           
  Lines        3926     3926           
=======================================
  Hits         2108     2108           
  Misses       1621     1621           
  Partials      197      197
Impacted Files Coverage Δ
protoc-gen-grpc-gateway/gengateway/template.go 65.51% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87b57f5...5bf89d8. Read the comment docs.

@happyalu
Copy link
Contributor Author

happyalu commented Mar 6, 2019

Could you try building the grpc-gateway generator off this branch and see if it fixes tmc/grpc-websocket-proxy#14 for you?

Yes, my websocket client is working again.

@johanbrandhorst johanbrandhorst merged commit be12715 into grpc-ecosystem:master Mar 6, 2019
@johanbrandhorst
Copy link
Collaborator

Thank you so much for this contribution!

@happyalu happyalu deleted the happyalu/bugfix/readAll branch March 7, 2019 02:34
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
An IOReaderFactory was being used to wrap request body for client/bidi streaming
requests.  This was causing the requests to be fully buffered before being sent
to the grpc server, thereby breaking streaming.  This commit changes that to
directly use request body.

Fixes grpc-ecosystem#894
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants